Skip to content

Fix LogLevel: CLI phase suppression and case-insensitive "Default" config key#3203

Merged
Aniruddh25 merged 17 commits intomainfrom
copilot/fix-loglevel-incorrect-behavior
Mar 13, 2026
Merged

Fix LogLevel: CLI phase suppression and case-insensitive "Default" config key#3203
Aniruddh25 merged 17 commits intomainfrom
copilot/fix-loglevel-incorrect-behavior

Conversation

Copy link
Contributor

Copilot AI commented Mar 7, 2026

Why make this change?

  • [Bug]: LogLevel incorrect behavior and failing #3201
    Two LogLevel bugs: Using the None LogLevel still emits some Information messages (version, config path, etc.), and using "Default" (capital D) as a key in telemetry.log-level config crashes startup with NotSupportedException.

What is this change?

All logs follow the LogLevel configuration

  • Added more specific configuration for logs in the host level, Program.cs which where outputing some logs that where not following the LogLevel configuration from CLI and from configuration file.
  • Add DynamicLogLevelProvider class which allows the loggers in the host level to be updated after the RuntimeConfig is parsed and we know the LogLevel.
  • Add StartupLogBuffer class which saves the logs that are created before we know the LogLevel, and sends them to their respective logger after the RuntimeConfig is parsed.

Case-insensitive "Default" key in config

  • IsLoggerFilterValid used string.Equals (ordinal), so "Default" failed against the registered "default" filter.
  • GetConfiguredLogLevel used TryGetValue("default") (case-sensitive), silently ignoring "Default" keys.
  • Both fixed with StringComparison.OrdinalIgnoreCase / LINQ FirstOrDefault.
// Now works — previously threw NotSupportedException
"telemetry": {
  "log-level": {
    "Default": "warning"
  }
}
# Now silent — previously emitted "Information: Microsoft.DataApiBuilder ..."
dab start --LogLevel None

How was this tested?

  • Integration Tests
  • Unit Tests
    • ValidStringLogLevelFilters: added "Default" (capital D) data row to cover the case-insensitive validation fix.

Sample Request(s)

# Suppress all output
dab start --LogLevel None

# Config key now case-insensitive
# dab-config.json:
# "telemetry": { "log-level": { "Default": "none" } }
dab start

…ive Default key

Co-authored-by: RubenCerna2079 <32799214+RubenCerna2079@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix incorrect behavior of LogLevel Fix LogLevel: CLI phase suppression and case-insensitive "Default" config key Mar 7, 2026
@RubenCerna2079 RubenCerna2079 linked an issue Mar 9, 2026 that may be closed by this pull request
@Aniruddh25 Aniruddh25 added the 2.0 label Mar 10, 2026
@RubenCerna2079
Copy link
Contributor

/azp run

@RubenCerna2079 RubenCerna2079 marked this pull request as ready for review March 12, 2026 00:24
Copilot AI review requested due to automatic review settings March 12, 2026 00:24
@azure-pipelines
Copy link

Azure Pipelines successfully started running 6 pipeline(s).

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Addresses two logging-related startup issues: (1) honoring --LogLevel during early/CLI-phase output, and (2) treating the telemetry.log-level config key "default" case-insensitively so "Default" doesn’t crash startup.

Changes:

  • Updates runtime-config log-level validation and lookup to be case-insensitive for "default"/"Default".
  • Refactors service startup/config-loader logging to route messages through logging infrastructure (with buffering intended before final log level is known).
  • Adjusts CLI E2E tests around --LogLevel behavior, splitting Information-and-below vs Warning-and-above scenarios.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 10 comments.

Show a summary per file
File Description
src/Service/Startup.cs Adds log buffering + runtime log-level update wiring during startup (and config-loader logger wiring).
src/Service/Program.cs Adds dynamic filtering hooks intended to suppress early messages based on effective log level.
src/Config/FileSystemRuntimeConfigLoader.cs Replaces direct console writes with logger/buffered logging and adds logger setter/flush.
src/Config/ObjectModel/RuntimeConfig.cs Makes "default" log-level lookup case-insensitive in GetConfiguredLogLevel.
src/Core/Configurations/RuntimeConfigValidator.cs Makes logger-filter validation case-insensitive.
src/Service.Tests/Configuration/ConfigurationTests.cs Adds a test row covering "Default" log-level key validation.
src/Cli/Program.cs Keeps CLI logger factory usage and loader creation (but currently still doesn’t wire args into log level).
src/Cli.Tests/EndToEndTests.cs Updates E2E coverage for --LogLevel expectations and adds a Warning+ startup test.

You can also share your feedback on Copilot code review. Take the survey.

@Aniruddh25
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 6 pipeline(s).

@aaronburtle
Copy link
Contributor

aaronburtle commented Mar 12, 2026

Is the description up to date?

Added Program.PreParseLogLevel() — scans raw args[] for --LogLevel before CommandLine.Parser runs — so the logger factory is configured at the right level before any CLI-phase output is emitted.

I dont see these changes in the files, or was this renamed?

@aaronburtle
Copy link
Contributor

CLI logger respects --LogLevel CustomConsoleLogger had a hardcoded _minimumLogLevel = LogLevel.Information, making it ignore the --LogLevel flag entirely.

How is the CustomConsoleLogger getting --loglevel? I think some changes were reverted and maybe the description isn't accurate anymore.

Copy link
Contributor

@aaronburtle aaronburtle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good once the description is fixed.

@RubenCerna2079
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 6 pipeline(s).

Copy link
Contributor

@aaronburtle aaronburtle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!

@RubenCerna2079 RubenCerna2079 enabled auto-merge (squash) March 12, 2026 22:10
@RubenCerna2079 RubenCerna2079 disabled auto-merge March 12, 2026 22:13
@anushakolan
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 6 pipeline(s).

@Aniruddh25 Aniruddh25 merged commit abcfb58 into main Mar 13, 2026
11 checks passed
@Aniruddh25 Aniruddh25 deleted the copilot/fix-loglevel-incorrect-behavior branch March 13, 2026 04:56
@github-project-automation github-project-automation bot moved this from Todo to Done in Data API builder Mar 13, 2026
RubenCerna2079 added a commit that referenced this pull request Mar 13, 2026
…nfig key (#3203)

## Why make this change?

- #3201 
Two `LogLevel` bugs: Using the `None` LogLevel still emits some
`Information` messages (version, config path, etc.), and using
`"Default"` (capital D) as a key in `telemetry.log-level` config crashes
startup with `NotSupportedException`.

## What is this change?

**All logs follow the LogLevel configuration**
- Added more specific configuration for logs in the host level,
`Program.cs` which where outputing some logs that where not following
the LogLevel configuration from CLI and from configuration file.
- Add `DynamicLogLevelProvider` class which allows the loggers in the
host level to be updated after the RuntimeConfig is parsed and we know
the LogLevel.
- Add `StartupLogBuffer` class which saves the logs that are created
before we know the LogLevel, and sends them to their respective logger
after the RuntimeConfig is parsed.

**Case-insensitive `"Default"` key in config**
- `IsLoggerFilterValid` used `string.Equals` (ordinal), so `"Default"`
failed against the registered `"default"` filter.
- `GetConfiguredLogLevel` used `TryGetValue("default")`
(case-sensitive), silently ignoring `"Default"` keys.
- Both fixed with `StringComparison.OrdinalIgnoreCase` / LINQ
`FirstOrDefault`.

```json
// Now works — previously threw NotSupportedException
"telemetry": {
  "log-level": {
    "Default": "warning"
  }
}
```

```
# Now silent — previously emitted "Information: Microsoft.DataApiBuilder ..."
dab start --LogLevel None
```

## How was this tested?

- [ ] Integration Tests
- [x] Unit Tests
- `ValidStringLogLevelFilters`: added `"Default"` (capital D) data row
to cover the case-insensitive validation fix.

## Sample Request(s)

```bash
# Suppress all output
dab start --LogLevel None

# Config key now case-insensitive
# dab-config.json:
# "telemetry": { "log-level": { "Default": "none" } }
dab start
```

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: RubenCerna2079 <32799214+RubenCerna2079@users.noreply.github.com>
Co-authored-by: Ruben Cerna <rcernaserna@microsoft.com>
Co-authored-by: Aniruddh Munde <anmunde@microsoft.com>
Co-authored-by: Anusha Kolan <anushakolan10@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

[Bug]: LogLevel incorrect behavior and failing

7 participants